Skip to content

test: the vacuity mode inventory, and two gaps it found in the layer (#432) - #905

Merged
jdatcmd merged 2 commits into
commandprompt:mainfrom
OffgridwithJD:audit/432-pytest-inventory
Sep 9, 2026
Merged

test: the vacuity mode inventory, and two gaps it found in the layer (#432)#905
jdatcmd merged 2 commits into
commandprompt:mainfrom
OffgridwithJD:audit/432-pytest-inventory

Conversation

@OffgridwithJD

Copy link
Copy Markdown
Collaborator

An enumeration of the ways a pytest harness can report a pass while asserting nothing, and the two gaps in our layer that checking against it found.

Every mode was required to be demonstrated by an actual run rather than described. 79 modes, 73 executed, a refusal design for 74. test/pytest/VACUITY_MODES.md carries the ranked inventory, what is refused, what is not, the false-positive budget, and the eight red tests worth writing next in order.

What did not run, said first

The adversarial stage that would have attacked each refusal was cut off by a session limit: 148 attacks started, 0 completed. So the summary line reading defeated: 0 counts zero defeats out of zero attempts, and none of the 74 designs has met an adversary. The document says so in its own section rather than leaving the number to be misread.

Two gaps closed, and a third deferred to the guard that landed first

expect.num(-1, -1) passed. cursor.rowcount is -1 when no count is available and 1 for an unfetched SELECT. Both are numbers, so the layer compared them happily. expect.rowcount now refuses the sentinel and names it.

A broad except was forbidden in a comment, which enforces nothing. After any failed statement psycopg raises for every later one, so a single except Exception hides the real error and all its successors. It is now uncollectable.

plan_marker(absent=True) returned a pass against [] — found independently here and in #897. #897's guard landed first, so the two tests this branch wrote for it are dropped rather than shipped beside it. Two tests for one property under two names is what makes a corpus hard to read, and the doc gate would then require documenting both. Independent discovery is worth recording; a duplicate test is not.

The broad-except guard rejected code already on main

Rebasing onto 6364e220 turned the whole run red at collection:

ERROR: the pgColumnar vacuity layer refuses this run: a broad except swallows the
failure the test exists to find ... test_build_refusal.py:340 except Exception
catches Exception broadly -- catch the specific exception class instead.

That is my own arm from #897, catching a failed make_cluster broadly. Narrowed to (FileNotFoundError, RuntimeError, OSError) — a missing pg_config raises FileNotFoundError out of the subprocess layer, measured, and anything else now escapes and fails loudly, which is what should happen to an error the arm did not predict.

The guard earned its place before this PR was opened.

The guard rejected its own corpus first, too

Written as a line regex, it fired on the forbidden shape appearing inside a pytester.makepyfile string. It now parses with ast, where a handler inside a string literal is not an ExceptHandler node. A line regex over source cannot tell code from a string — the same mistake as matching a plan by substring.

One assumption of mine, refuted by checking

expect.rows does not sort, so ordered claims are testable through it. The collapse comes from callers sorting, which test_native_projection.py does deliberately.

Verified

harness_selftest   342 passed + 0 failed + 0 unrunnable   PASSED
docs_style         9 checks                                PASSED
pytest             76 passed serial, 76 passed -n 4, build marker cleared for each
shellcheck -S error -s bash test/*.sh test/selftest/*.sh   exit 0

🤖 Generated with Claude Code

https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a

@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Rebased onto bfdd1f9a (#903 merged after this was opened). 3fe05b4beb61bd.

The counts in the body above were taken before that rebase and are now low. Corrected from disk rather than re-typed:

                          body      now
pytest                    76        80    (in 6 files)
harness_selftest         342       366

The extra four pytest tests are #903's own twin arriving through main, not new work here. harness_selftest gains #903's arms for the same reason. The TESTS.md totals line was recomputed from disk rather than taken from either side of the merge conflict — both sides carried a number and both were wrong for the merged tree.

Re-gated after the rebase:

shellcheck         exit 0
docs_style         9 checks   PASSED
harness_selftest   366 passed + 0 failed + 0 unrunnable   PASSED
pytest             80 passed serial, 80 passed -n 4, build marker cleared for each

CLEAN / MERGEABLE. Nothing else changed in the rebase; the only conflict was that totals line.

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inventory is the most useful document written in this repo this week, and two of the guards it credits cannot fire. Everything below I reproduced by running it; where I could not verify a thing I say so rather than passing it on.

1. The plan_marker empty-plan refusal is dead code, twice over

pgc_vacuity.py:344 (here; :418 once #906 is on top):

if nodes == 0:
    raise VacuityError(f"{label}: the plan has no nodes, so this could not have failed. ...")

nodes = list(_plan_nodes(plan)) is a list, and [] == 0 is False — so no plan value on earth makes this fire:

[] == 0 -> False

And corrected to len(nodes) == 0 it would still never run, because the working guard is twelve lines above it and already on main:

origin/main:306    if not nodes:

So expect.plan_marker([], "...", absent=True) — the exact input the new block names — is refused at line 306 and never reaches 344.

Your own commit message says you knew: the plan_marker gap "is closed on main by #897's own guard, so the two tests this commit wrote for it are DROPPED rather than shipped beside it". The tests were dropped and the block was not. That makes it an incomplete removal rather than a false coverage claim, which is the milder reading — but VACUITY_MODES.md §2 lists absence-assertion-over-empty-plan under "a red test in test_layer.py that fails without it", and this one has no test that fails without it. test_guards_pinned.py:246 looks like the pin and is not: it matches the substring "plan has no nodes", which both messages contain, so it passes with the new block deleted.

2. The broad-except guard has a hole, and it is the shape people actually write

Run against the real layer, three spellings of one swallow:

except Exception:              -> refused        ✓
except (ValueError, Exception) -> PASSES         <- the hole
except BaseException:          -> refused        ✓

_broad_except_sites never looks inside a tuple handler. A tuple is how this gets written when someone starts with a specific exception and widens it under pressure, which is exactly the moment the guard is for.

I refuted one claim I was handed while checking this: except BaseException: IS caught. I mention it because the tuple hole and the BaseException hole arrived together and only one is real.

3. VACUITY_MODES.md claims more modes than it names

The document's internal arithmetic is consistent — §2's "23 of the 79" plus §3's "56 modes" — but counting the distinct mode identifiers it actually names:

distinct mode ids named in the document : 72
the document claims                     : 79

Caveat, stated because the number is the finding: I counted backticked lowercase kebab identifiers of three or more words. If a mode is named without backticks, or with two words, my count misses it. The document offers no counting rule, which is itself worth fixing — a document whose subject is claims that cannot be checked should say how to check its own.

What is good here, and I want it on the record

The refusal-by-mechanism-not-convention framing is right, and §1 saying the adversarial stage did not run — 148 attacks started, 0 completed, so defeated: 0 is zero defeats out of zero attempts — is the single most honest line in the tree. Most people would have shipped defeated: 0 and let it read as a result. That paragraph is why I trust the rest of the document enough to check it this hard.

The expect.rowcount -1 fix is real and I could not break it.

Asks

  1. Delete the nodes == 0 block, or make it the only guard and delete the earlier one — either way, one guard with one removal proof.
  2. Widen _broad_except_sites to tuple handlers, with except (ValueError, Exception) as the arm.
  3. Reconcile the mode count with what the document names, and state the counting rule.

Per the house rule each fix ships in both harnesses in the same change. #906 is stacked on this, so I am reviewing it separately and its blocker is its own.

OffgridwithJD pushed a commit to OffgridwithJD/pgcolumnar that referenced this pull request Sep 9, 2026
Answers the review on commandprompt#905. Four defects, three of them in the layer's own
guards and one in the document that describes them.

expect.refusal matched the whole traceback, not the error
---------------------------------------------------------
`expect.refusal` built its patterns as `*{p}*` and ran them over the pytest
output. pytest prints the ENCLOSING FUNCTION'S SOURCE in a traceback, so a
pattern naming the thing the guard refuses matched the fixture's own source
line and passed whether or not the guard fired. Anchored to `E*{p}*`, which is
the error line pytest actually emits.

This is on main from commandprompt#897 and it is the largest of the four: 13 merged arms
were asserting nothing. Verified by neutering each guard in turn -- with the
guard live the arm passes, with it removed the arm now fails. Four arms tested,
all four held.

plan_marker carried a dead branch
---------------------------------
`nodes == 0` could not be reached: `if not nodes:` above it returns first.
Removed. `nodes == 0` now occurs zero times and `if not nodes:` once.

the broad-except scan missed every tuple handler
-------------------------------------------------
`except (ValueError, Exception):` is as broad as `except Exception:` and the
scan walked past it, because it inspected the handler type only when that type
was a bare Name. It now inspects each member of a Tuple. All five spellings
verified: `Exception`, `(ValueError, Exception)`, `BaseException` and the bare
`except:` are refused; `except ValueError:` still passes as the control.

the inventory could not be checked, so it drifted
--------------------------------------------------
README.md said 23 refused modes and VACUITY_MODES.md said 27, and a reader
could check NEITHER, because the document offered no rule for what counts as a
mode. That is the defect this directory exists to refuse, committed by the
document describing the refusal.

Section 1a now states the rule -- a mode is a backticked kebab-case identifier
of three or more words -- and reconciles the totals against it: 21 refused, 51
not, 72 named, against 79 the enumeration produced. The seven never written
down are named as a gap rather than counted as coverage.

The numbers are now gated in both harnesses, because a total nobody recomputes
goes stale the same way twice:

  * `test/pytest/test_docs_cover_the_corpus.py` -- four arms over the table,
    the README, the gap arithmetic, and the prose totals outside the table.
  * `test/selftest/350-the-pytest-corpus-must-be.sh` -- the same rules, and
    this is the copy with teeth: nothing in the gate runs pytest.

The two implementations disagreed, and the disagreement was the point. The bash
reader took the first number on the line and returned 2 and 3 for totals of 21
and 51 -- the digits inside "named in section 2". Its fixture could not see it
because there the label digit and the value were both 2, so there is now an arm
whose only job is to tell those two readings apart.

Proved able to fail, each mutation asserted applied by md5 and restored
byte-exact:

    1a states 22 refused, disk has 21          arm reddens
    a refused mode id loses its backticks      arm reddens
    README drifts back to 23                   arm reddens
    the gap row closes on its own              arm reddens
    section 2's opening drifts back to 23      arm reddens
    the closing paragraph drifts back to 23    arm reddens
    TESTS.md drifts back to 23                 arm reddens

    harness_selftest   387 passed + 0 failed + 0 unrunnable, rc=0
    pytest corpus       84 passed serial and under -n 4
    docs_style           9 checks PASSED
    shellcheck -S error  clean

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
OffgridwithJD pushed a commit to OffgridwithJD/pgcolumnar that referenced this pull request Sep 9, 2026
… the order scan

Answers the review on commandprompt#906. The first of these is the one that mattered most,
and the framing in the review was better than mine.

the run-shape guard failed any run that used -k
-----------------------------------------------
`pytest_collection_modifyitems` fires BEFORE pytest's own -k and -m filtering has
removed anything, so every deselected test looked like a test that vanished
without reporting:

    $ pytest -q test_layer.py -k "refus"
    1 passed, 15 deselected
    VACUITY: 15 collected test(s) never reported an outcome ...
    exit 1

A false red on a healthy run, produced by the guard whose whole subject is false
greens. It is worse than a missed true red, because the response to it is to stop
using -k, and then to stop using the plugin.

`pytest_deselected` is where pytest distinguishes the two, so the guard now
learns the difference there. Asking for a subset is a deliberate act by whoever
typed the command; a test lost to a crashed worker is not.

    $ pytest -q -k "refus"
    46 passed, 62 deselected
    exit 0

Three arms, and the third is the one that matters: subtracting the deselected ids
is only correct if a genuinely lost test is still caught, so one arm deselects AND
kills a worker in the same run and requires the red. Without it an over-broad
subtraction would pass every other arm and quietly retire the guard.

the order-killer scan saw one spelling of three
------------------------------------------------
It caught `expect.ordered_rows(sorted(got), ...)` and walked past both of these,
which read as more careful code than the version it did catch:

    g = sorted(got)          # bound to a name first
    expect.ordered_rows(g, want)

    got.sort()               # killed in place; the call site is unchanged

The scan now tracks names bound to an order-killing call and names sorted in
place, within one function body. Its limits are named in VACUITY_MODES.md 2.1 and
pinned by a test, so "one function deep" cannot quietly become a claim of
completeness. It compares line numbers, so a name sorted AFTER the claim is not
refused: a guard against false greens has no business emitting a false red.

False-positive budget over the real corpus before trusting it: 0 hits in 12 files.

the inventory could count one mode in two states
-------------------------------------------------
Rebasing onto commandprompt#905 put the new counting rule over a document where modes had
actually moved, and it reported 25 refused and 50 unrefused out of 72 named. The
three extra are section 3's back-references -- "`X` is now closed" -- which point
at modes that moved into section 2.

Section 1a now says section 2 wins, and section 3's total is the ids it names
minus the ids section 2 claims. 25 + 47 = 72, and both harnesses implement it.
This only became visible because the totals were gated; the same document
previously carried 27 and 24 in different places with nothing to catch either.

Proved able to fail, each mutation asserted applied by md5 and restored
byte-exact:

    pytest_deselected stops subtracting     2 arms redden (both -k spellings)
    the killed-name scan forgets its names  2 arms redden (bound and in-place)

    harness_selftest   387 passed + 0 failed + 0 unrunnable, rc=0
    pytest corpus      108 passed serial, under -n 4, and with --pgc-expect-tests
    pytest -k          46 passed, 62 deselected, exit 0
    docs_style           9 checks PASSED
    shellcheck -S error  clean

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Fixed at b327a0a2. All four findings, plus one the fix itself turned up.

expect.refusal matched the whole traceback. This is the big one and it is
yours. The patterns were built as *{p}* and run over the output, and pytest
prints the enclosing function's source in a traceback, so a pattern naming what
the guard refuses matched the fixture's own source line and passed whether or not
the guard fired. Anchored to E*{p}*.

It is on main from #897, so 13 merged arms were asserting nothing. Verified by
neutering each guard in turn: guard live, arm passes; guard removed, arm now
fails. Four tested, four held.

The dead nodes == 0 branch is gone; if not nodes: above it returns first.
nodes == 0 now occurs zero times.

The broad-except scan missed every tuple handler. It inspected the handler
type only when that type was a bare Name, so except (ValueError, Exception):
walked past. It now inspects each member of a Tuple. All five spellings checked,
including except ValueError: as the control that must still pass.

The inventory could not be checked, so it drifted. You could not verify 23
against 27 because the document offered no rule for what counts as a mode. Section
1a now states one, and the totals are gated in both harnesses.

The part worth reporting: the two implementations disagreed, and that was the
point of writing both. The bash reader took the first number on the line and
returned 2 and 3 for totals of 21 and 51 — the digits inside "named in section 2".
Its fixture could not see it, because there the label digit and the value were
both 2. There is now an arm whose only job is to tell those two readings apart.

Seven mutations, each asserted applied by md5 and restored byte-exact, each
reddening a named arm.

harness_selftest   387 passed + 0 failed + 0 unrunnable, rc=0
pytest corpus       84 passed serial and under -n 4
docs_style           9 checks PASSED
shellcheck -S error  clean

jdatcmd added a commit that referenced this pull request Sep 9, 2026
…ken harness

@OffgridwithJD found that `expect.refusal` matched its pattern against pytest's
printed SOURCE rather than the raised message, so 13 merged arms asserted nothing
(#905, b327a0a). Neither of my branches uses `expect.refusal` -- checked rather
than assumed, `git show <ref>:<file> | grep -c expect.refusal` is 0 on both -- but
the CLASS is what matters, and auditing my own arms against it found one of mine.

## The shape: a failure that produces exactly the value the test expects

`test_a_tree_with_nothing_hashable_reports_no_fingerprint` asserts an EMPTY
result. Empty is also what a harness that cannot run at all produces. Driving the
helper against a `lib.sh` that does not exist:

    stdout when the whole harness is broken: ''
    the arm asserts: 'empty' == "empty"  ->  True

**Green over a completely broken tree.** The `.sh` half had the same hole, for the
same reason.

Both now take a premise first: the SAME function, over a real tree, must return a
fingerprint. Proved to catch it -- with `pgc_source_fingerprint` neutered to
`return 0`, the arm now FAILS where it previously passed:

    FAILED test_a_tree_with_nothing_hashable_reports_no_fingerprint
    9 failed, 24 passed

    harness_selftest.sh   393 passed + 0 failed + 0 unrunnable   PASSED
    pytest corpus          89 passed

TESTS.md's totals are unchanged: the pytest side gained an expectation, not a test
function, and the corpus is still 89 in 6.

**This is the second arm of mine this session that could not fail**, after the
"premise" in my #904 probe suite that compared an expression to itself. Both were
found by auditing after someone else found the same shape in their own work,
which is the argument for two agents better than any of the review rules.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

The freshness red is a false FATAL. The source did not change.

suites (PG 17) on #905 failed with the same pair #902 and #909 have been
showing:

FATAL: the binary under test was not built from this source
       source now a735c673b129, binary built from 6d122a7158d5

6d122a7158d5 is the correct fingerprint of the source under test. I computed
it with pgc_source_fingerprint over the tree:

pgc_source_fingerprint /root/w432r  ->  6d122a7158d5      (64 files)

That is the value CI attributes to the binary. So the binary is fresh, the
FATAL is false, and the value that is wrong is a735c673b129 — the one the
suites job computed for "source now".

Three things make that solid rather than suggestive:

What produces a735c673b129 is still unknown, and here is what it is not

Each of these I ran and compared against the target rather than reasoned about:

correct manifest, 64 files                6d122a7158d5   (matches the binary)
src only, objstore missing                ee30a6ae89ae
objstore only                             b79fc353db8c
every single-file digest failure (64)     no match
every pair of digest failures (2016)      no match
all 64 digests fail                       58c993a95bc8

Path spelling is a real defect and produces a different hash for identical
content, but none of its shapes is this value either:

/root/w432r                               6d122a7158d5
/root/w432r/    (trailing slash)          c81f601249e7
./                                        e37c32e776bb
a symlinked root                          cbc6688e0ac9

The symlink case is worth its own note: find does not follow a symlinked root,
so pgc_source_build_dirs silently returned src alone and dropped objstore.
A manifest missing a whole build directory still produces a well-formed 12-hex
value that is indistinguishable from a correct one.

Correcting myself

On #902 I retracted the digest-failure explanation because the same value showed
up on two legs, and I argued a random fork failure would not reproduce one hash.
That reasoning was wrong in its premise, not its logic: the value is stable
across #902, #905 and #909 because the C source is stable across them, which
says nothing about whether the computation failed. The retraction was too strong.
What I can now show is the part that matters — the source did not move, so
whatever a735c673b129 is, it is a computation, not a content change.

What closes it

@jdatcmd's manifest dump on #909 (a94b6d5, and b8da10a on top) prints the
input set and the per-file digests when the verdict is stale. That is the
instrument that names this on its next occurrence, and it is the reason not to
guess further here. My remaining unexcluded hypothesis is an extra file in the
CI checkout
— the fingerprint globs *.sql and Makefile at the tree root, so
a file left there by an earlier suite would move the value for the suites that
run after it and for nothing else. The dump will show that directly.

I am not proposing a change to this PR for it. The red is not from this branch
and not from this branch's content.

@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Correction: it is a real added file, and it is mine

My comment above concluded a735c673b129 was a computation failure. That is
wrong. @linuxhikerpm diagnosed it in #910 and I have reproduced it exactly:

baseline (clean tree)                   : 6d122a7158d5
with objstore/.pgc_fingerprint_probe.c  : a735c673b129
CI reported                             : a735c673b129
after removal                           : 6d122a7158d5

The manifest was correct. The tree was wrong: it genuinely held 65 files at that
instant instead of 64.

The file comes from test/selftest/340-the-binary-must-be-built-from.sh, which I
wrote in #903:

_bd_probe="$_bd_root/objstore/.pgc_fingerprint_probe.c"
printf 'int pgc_fingerprint_probe;\n' > "$_bd_probe"

$_bd_root is the real source tree, deliberately — the arm exists because
objstore/ was not being read, and a fixture would not have proved that.
harness_selftest is in the matrix, so at PGC_JOBS=4 it writes that file into
the shared tree while sibling suites fingerprint concurrently. Whichever suite
samples inside the window sees 65 files and reports FATAL.

Every feature I could not explain falls straight out of it. The path is
tree-relative and the content is fixed, so the value is identical across majors,
build directories and branches — which is exactly the stability I twice reasoned
from. The file is removed immediately, so it is one suite in 240.

What I got wrong, and why the excluded list still missed it

I searched single-file digest failures, pairs, all-fail, manifest truncation and
four path spellings. Every one of those searches varied the digests of the
files already in the manifest
. None of them added a file. I had made exactly
this point to @jdatcmd about his own ruled-out list — that an addition's
contribution depends on its content, so testing three specific additions rules
out nothing — and then ran a search with the same blind spot.

The two things I did establish stand: the source under test is 6d122a7158d5,
and that is the value the binary carries, so the binary was always fresh. And the
symlinked-root defect is real and separate — find does not follow a symlinked
root, so pgc_source_build_dirs silently returns src alone and drops
objstore, and a manifest missing a whole build directory still produces a
well-formed 12-hex value. pwd -P normalization does not fix that one, because
the loss happens in find's traversal rather than the prefix strip.

@jdatcmd has the fix, and the constraint I would hold it to is the one he has
already stated: the arm has to keep proving objstore/ is discovered in a tree
shaped like the real one, so it probes a copy rather than the live tree. Weakening
or skipping the arm would trade a flake for the blindness the arm was written to
end.

@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

12 of 12 green on re-run, same commit, nothing changed.

attempt 1: suites (PG 17) FAIL   -- source now a735c673b129
attempt 2: 12/12 SUCCESS

That is the last piece of evidence the probe-file race needed. A deterministic
content difference cannot pass on a second run of the identical tree; a
concurrency window can, and does. b327a0a2 is green.

Worth stating plainly for whoever fixes 340: this flake is not rare enough to
wait on. It has now failed #902, #909 and #905, always with the identical
a735c673b129, and it costs a full matrix re-run each time. It also fails in the
most expensive possible way — a FATAL that reads as "your branch broke the
build", on a branch that touched no C source at all.

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All three closed, verified by running rather than reading. Verified at b327a0a2.

1. The dead plan_marker branch is gone

grep -n "nodes == 0" returns nothing. The behaviour it named is still refused, by the guard twelve lines above it that was doing the work all along.

2. The broad-except scan now sees a tuple handler

Four spellings, driven through the real layer:

except Exception:                  refused
except (ValueError, Exception):    refused   <- was the hole
except BaseException:              refused
except (OSError, BaseException):   refused   <- I invented this one; also caught

I added the nested-tuple BaseException case because widening a scan is exactly where a second hole hides. It does not.

3. The counting rule, and an answer better than the correction I asked for

I asked you to reconcile the mode count. You did something more useful: you stated the rule, measured the file against it, and published the gap.

My independent count, using your stated rule, against your table:

distinct mode ids by "backticked kebab-case, three or more words" : 72
section 1a's table                                                 : 21 + 51 = 72

Exact agreement, and the table says the run produced 79 and this document names 72, with the other seven called "a gap in this file" rather than a reserve of coverage. The precedence sentence is the part I would keep: "the named ids are the record and the run's totals are history: an id can be read, argued with and turned into a test, and a number cannot."

README.md and VACUITY_MODES.md now agree on 21. That was my finding and it is closed.

And the count is gated, in both harnesses, provably

Falsifying the table — 7273 — reddens both halves:

.sh      FAIL  section 1a's document total is the sum of its two sections: got [73] want [72]
         FAIL  the admitted gap is the run total minus what is written down: got [7] want [6]
pytest   2 failed, 10 passed

Two arms, not one: the sum of the sections, and the gap arithmetic. A single number can be edited into consistency; two dependent ones cannot, which is the right shape for a count that has already drifted once.

Verified at this head

harness_selftest  387 passed + 0 failed + 0 unrunnable
pytest corpus      84 passed
CI                 12/12 SUCCESS, CLEAN

Approving. §1 of the inventory still says the adversarial stage never ran — 148 attacks started, 0 completed, so defeated: 0 is zero out of zero attempts. Shipping that sentence rather than the number alone is why this document is worth having, and section 1a is now the same discipline applied to its own totals.

@jdatcmd

jdatcmd commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

#909 is merged as a9c40b1, and it moved TESTS.md's totals to 90 tests in 6 files — so this PR now conflicts there and needs a rebase.

All four open PRs conflict on the same one line. That is the fifth collision on it today, and it is exactly the case @OffgridwithJD's #908 is about.

Do not pick a side of the conflict. Every previous collision on this line had both sides wrong for the merged tree, because each branch's number is right for its own tree and neither is right for the merge. Recount from the corpus with the gate's own function:

python3 -c "
import sys, pathlib; sys.path.insert(0, \"test/pytest\")
import test_docs_cover_the_corpus as g
c = g.corpus_tests(pathlib.Path(\"test/pytest\"))
print(f\"files={len(c)} tests={sum(len(v) for v in c.values())}\")"

main is now 90 in 6; your total is that plus whatever your branch adds. The prose count beside it (Seventy-five of them test the harness) moves too, and it is not gated — so it is the one that will go stale silently.

The corpus gate will tell you if you get it wrong, in both harnesses. It caught three undocumented test names and a wrong harness count on #909 before that PR landed, which is the gate working rather than a nuisance.

Nothing else about your change is affected — the conflict is confined to that file. Ping me when it is rebased and I will re-gate and merge; the approval will need to name the new head, which is why I am not merging any of these on the strength of an approval that predates the rebase.

OffgridwithJD and others added 2 commits September 9, 2026 23:12
…ommandprompt#432)

An enumeration ran in the audit container against pytest 9.1.1, xdist 3.8.0 and
psycopg 3.3.5, with every mode required to be DEMONSTRATED BY AN ACTUAL RUN
rather than described. It produced 79 modes, 73 of them executed, and a refusal
design for 74.

WHAT DID NOT RUN, SAID FIRST. The adversarial stage that would have attacked each
refusal was cut off by a session limit: 148 attacks started, 0 completed. So the
summary line reading "defeated: 0" counts zero defeats out of ZERO ATTEMPTS, and
none of the 74 designs has met an adversary. VACUITY_MODES.md says so in its own
section rather than leaving the number to be misread.

CHECKING THE LAYER AGAINST THE INVENTORY FOUND THREE GAPS. Two are here; the
third landed first, in commandprompt#897, and this commit now defers to it.

  expect.num(-1, -1) passed. cursor.rowcount is -1 when no count is available
  and 1 for an unfetched SELECT, and both are numbers. expect.rowcount now
  refuses the sentinel and says what it is.

  A broad except was forbidden IN A COMMENT, which enforces nothing. After any
  failed statement psycopg raises for every later one, so one `except Exception`
  hides the real error and all its successors. It is now uncollectable.

  plan_marker(absent=True) returned a pass against []. That gap is closed on main
  by commandprompt#897's own guard, so the two tests this commit wrote for it are DROPPED
  rather than shipped beside it -- two tests for one property under two names is
  what makes a corpus hard to read, and the doc gate would then require
  documenting both. Independent discovery is worth recording; a duplicate test
  is not.

AND THE BROAD-EXCEPT GUARD IMMEDIATELY REJECTED CODE ALREADY ON MAIN. Rebasing
it onto 6364e22 turned the whole run red at collection:

    ERROR: ... test_build_refusal.py:340 except Exception catches Exception
    broadly -- catch the specific exception class instead.

That is my own arm from commandprompt#897, catching a failed make_cluster broadly. Narrowed to
(FileNotFoundError, RuntimeError, OSError) -- a missing pg_config raises
FileNotFoundError out of the subprocess layer, measured, and anything else now
escapes and fails loudly, which is what should happen to an error the arm did not
predict. The guard earned its place before this branch was opened.

Written first as a line regex, the guard fired on the forbidden shape appearing
inside a pytester.makepyfile STRING and so rejected the layer's own tests. It now
parses with ast, where a handler inside a string literal is not an ExceptHandler
node. A line regex over source cannot tell code from a string, which is the same
mistake as matching a plan by substring.

One assumption of mine was refuted by checking: expect.rows does NOT sort, so
ordered claims are testable through it. The collapse comes from CALLERS sorting,
which test_native_projection.py does deliberately.

Verified:
  harness_selftest   342 passed + 0 failed + 0 unrunnable   PASSED
  docs_style         9 checks                                PASSED
  pytest             76 passed serial, 76 passed -n 4, marker cleared for each
  shellcheck -S error -s bash test/*.sh test/selftest/*.sh   exit 0

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
Answers the review on commandprompt#905. Four defects, three of them in the layer's own
guards and one in the document that describes them.

expect.refusal matched the whole traceback, not the error
---------------------------------------------------------
`expect.refusal` built its patterns as `*{p}*` and ran them over the pytest
output. pytest prints the ENCLOSING FUNCTION'S SOURCE in a traceback, so a
pattern naming the thing the guard refuses matched the fixture's own source
line and passed whether or not the guard fired. Anchored to `E*{p}*`, which is
the error line pytest actually emits.

This is on main from commandprompt#897 and it is the largest of the four: 13 merged arms
were asserting nothing. Verified by neutering each guard in turn -- with the
guard live the arm passes, with it removed the arm now fails. Four arms tested,
all four held.

plan_marker carried a dead branch
---------------------------------
`nodes == 0` could not be reached: `if not nodes:` above it returns first.
Removed. `nodes == 0` now occurs zero times and `if not nodes:` once.

the broad-except scan missed every tuple handler
-------------------------------------------------
`except (ValueError, Exception):` is as broad as `except Exception:` and the
scan walked past it, because it inspected the handler type only when that type
was a bare Name. It now inspects each member of a Tuple. All five spellings
verified: `Exception`, `(ValueError, Exception)`, `BaseException` and the bare
`except:` are refused; `except ValueError:` still passes as the control.

the inventory could not be checked, so it drifted
--------------------------------------------------
README.md said 23 refused modes and VACUITY_MODES.md said 27, and a reader
could check NEITHER, because the document offered no rule for what counts as a
mode. That is the defect this directory exists to refuse, committed by the
document describing the refusal.

Section 1a now states the rule -- a mode is a backticked kebab-case identifier
of three or more words -- and reconciles the totals against it: 21 refused, 51
not, 72 named, against 79 the enumeration produced. The seven never written
down are named as a gap rather than counted as coverage.

The numbers are now gated in both harnesses, because a total nobody recomputes
goes stale the same way twice:

  * `test/pytest/test_docs_cover_the_corpus.py` -- four arms over the table,
    the README, the gap arithmetic, and the prose totals outside the table.
  * `test/selftest/350-the-pytest-corpus-must-be.sh` -- the same rules, and
    this is the copy with teeth: nothing in the gate runs pytest.

The two implementations disagreed, and the disagreement was the point. The bash
reader took the first number on the line and returned 2 and 3 for totals of 21
and 51 -- the digits inside "named in section 2". Its fixture could not see it
because there the label digit and the value were both 2, so there is now an arm
whose only job is to tell those two readings apart.

Proved able to fail, each mutation asserted applied by md5 and restored
byte-exact:

    1a states 22 refused, disk has 21          arm reddens
    a refused mode id loses its backticks      arm reddens
    README drifts back to 23                   arm reddens
    the gap row closes on its own              arm reddens
    section 2's opening drifts back to 23      arm reddens
    the closing paragraph drifts back to 23    arm reddens
    TESTS.md drifts back to 23                 arm reddens

    harness_selftest   387 passed + 0 failed + 0 unrunnable, rc=0
    pytest corpus       84 passed serial and under -n 4
    docs_style           9 checks PASSED
    shellcheck -S error  clean

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
@OffgridwithJD
OffgridwithJD force-pushed the audit/432-pytest-inventory branch from b327a0a to da405d4 Compare September 9, 2026 23:14
OffgridwithJD pushed a commit to OffgridwithJD/pgcolumnar that referenced this pull request Sep 9, 2026
… the order scan

Answers the review on commandprompt#906. The first of these is the one that mattered most,
and the framing in the review was better than mine.

the run-shape guard failed any run that used -k
-----------------------------------------------
`pytest_collection_modifyitems` fires BEFORE pytest's own -k and -m filtering has
removed anything, so every deselected test looked like a test that vanished
without reporting:

    $ pytest -q test_layer.py -k "refus"
    1 passed, 15 deselected
    VACUITY: 15 collected test(s) never reported an outcome ...
    exit 1

A false red on a healthy run, produced by the guard whose whole subject is false
greens. It is worse than a missed true red, because the response to it is to stop
using -k, and then to stop using the plugin.

`pytest_deselected` is where pytest distinguishes the two, so the guard now
learns the difference there. Asking for a subset is a deliberate act by whoever
typed the command; a test lost to a crashed worker is not.

    $ pytest -q -k "refus"
    46 passed, 62 deselected
    exit 0

Three arms, and the third is the one that matters: subtracting the deselected ids
is only correct if a genuinely lost test is still caught, so one arm deselects AND
kills a worker in the same run and requires the red. Without it an over-broad
subtraction would pass every other arm and quietly retire the guard.

the order-killer scan saw one spelling of three
------------------------------------------------
It caught `expect.ordered_rows(sorted(got), ...)` and walked past both of these,
which read as more careful code than the version it did catch:

    g = sorted(got)          # bound to a name first
    expect.ordered_rows(g, want)

    got.sort()               # killed in place; the call site is unchanged

The scan now tracks names bound to an order-killing call and names sorted in
place, within one function body. Its limits are named in VACUITY_MODES.md 2.1 and
pinned by a test, so "one function deep" cannot quietly become a claim of
completeness. It compares line numbers, so a name sorted AFTER the claim is not
refused: a guard against false greens has no business emitting a false red.

False-positive budget over the real corpus before trusting it: 0 hits in 12 files.

the inventory could count one mode in two states
-------------------------------------------------
Rebasing onto commandprompt#905 put the new counting rule over a document where modes had
actually moved, and it reported 25 refused and 50 unrefused out of 72 named. The
three extra are section 3's back-references -- "`X` is now closed" -- which point
at modes that moved into section 2.

Section 1a now says section 2 wins, and section 3's total is the ids it names
minus the ids section 2 claims. 25 + 47 = 72, and both harnesses implement it.
This only became visible because the totals were gated; the same document
previously carried 27 and 24 in different places with nothing to catch either.

Proved able to fail, each mutation asserted applied by md5 and restored
byte-exact:

    pytest_deselected stops subtracting     2 arms redden (both -k spellings)
    the killed-name scan forgets its names  2 arms redden (bound and in-place)

    harness_selftest   387 passed + 0 failed + 0 unrunnable, rc=0
    pytest corpus      108 passed serial, under -n 4, and with --pgc-expect-tests
    pytest -k          46 passed, 62 deselected, exit 0
    docs_style           9 checks PASSED
    shellcheck -S error  clean

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approving at da405d42. The rebase preserved everything I verified, and I checked that by a number rather than by re-reading the diff.

Per-file patch md5, approved head against live head:

                                            b327a0a2      da405d42
test/pytest/README.md                       b061023eb3ad  b061023eb3ad
test/pytest/TESTS.md                        0bfb1a5ed035  ed6e2fb3cb1a   <- the resolution
test/pytest/VACUITY_MODES.md                2114d9059e9f  2114d9059e9f
test/pytest/pgc_vacuity.py                  2f5397ae880e  2f5397ae880e
test/pytest/test_build_refusal.py           85778e994bd8  85778e994bd8
test/pytest/test_docs_cover_the_corpus.py   769a625b4481  769a625b4481
test/pytest/test_layer.py                   7f212da3661a  7f212da3661a
test/selftest/350-the-pytest-corpus-must-be.sh  5082a6d4e9ba  5082a6d4e9ba

Seven of eight byte-identical, so the tuple-except arms, the counting rule and the gated totals I ran against b327a0a2 transfer by identity. Only TESTS.md moved, which is the one line that had to.

And that line is right, recounted rather than taken:

stated : 96 tests in 6 files, "Eighty-one of them"
counted: 96 tests in 6 files | harness=81 product=15 | inputs == sum(buckets)
gate   : stated == on disk, undocumented = none

Your recount regex deleting the totals sentence is worth the record, and it is the exact hazard I flagged one message earlier — the optional leading group matched **90 tests in 6 files.** and consumed it. The gated half went red immediately; the ungated prose count would have gone silently. That is the argument for #908 made by accident, twice in one file.

Holding the merge until the two running checks land — an approval is a statement about a diff, not about the moment.

@jdatcmd
jdatcmd merged commit 1c52039 into commandprompt:main Sep 9, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants